CAMEL-23264: Enhance Splitter EIP with chunking, error threshold, failure tracking, and watermark resume#22300
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
|
|
||
| Removed 2 deprecated methods in Java DSL for `throttler` EIP. | ||
|
|
||
| ==== Split EIP enhancements |
There was a problem hiding this comment.
The upgrade guide should only contain information for end users that upgrade existing Camel apps - since all of this is new functionality then this is not relevant here and should be removed
There was a problem hiding this comment.
Done — removed the entire Split EIP section from the upgrade guide.
Claude Code on behalf of Guillaume Nodet
davsclaus
left a comment
There was a problem hiding this comment.
This is some advanced features so its good they are marked as such.
Note that group is already there as a simple language function, but it can be easier to set group=10 on the EIP and its more tooling friendly.
oscerd
left a comment
There was a problem hiding this comment.
This is a strong, well-tested enhancement to the Splitter EIP, and I like that the new behavior is strictly opt-in (all new fields default to inactive, so existing routes are unaffected). The model → reifier → processor wiring and the generated files all look consistent. A few items:
- The upgrade-guide entry for the Split EIP enhancements was added to
camel-4x-upgrade-guide-4_19.adoc, but this ships in 4.21 — it should move tocamel-4x-upgrade-guide-4_21.adocso users upgrading to 4.21 actually find it. - The new public
SplitResultclass incore/camel-apiis missing a@since 4.21tag (the project requires@sinceon new camel-api public types; the nestedFailurerecord and the public accessors are new surface too). - The
SonarCloud Code Analysischeck is red — worth opening the report to confirm it's coverage/cognitive-complexity rather than a flagged smell, especially since the PR set out to reduce complexity. - Just to confirm the intended contract: when
errorThreshold/maxFailedRecordsis set, per-item exceptions are cleared from sub-exchanges (so a customAggregationStrategywon't see them; failures surface viaSplitResult). It's documented — calling it out so it's a deliberate choice.
Really nice work overall.
Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
@gnodet can you rebase this on main - would be good to get this ready for 4.21 and it benefits camel end users and also the AI use-cases that comes |
0c83e16 to
5274c1f
Compare
|
Rebased onto latest
All 67 Splitter enhancement tests pass ✅ Claude Code on behalf of Guillaume Nodet |
|
this should be since 4.22 |
|
@davsclaus Thanks for the review and approval! Good point about Claude Code on behalf of Guillaume Nodet |
|
@oscerd Thanks for the thorough review! Addressing each item:
Claude Code on behalf of Guillaume Nodet |
|
@gnodet camel 4.21 has already been released, this code will go into the next release 4.22.0 |
There was a problem hiding this comment.
Pull request overview
This PR enhances Camel’s core Splitter EIP with first-class support for chunking (group), failure-tolerant processing (errorThreshold, maxFailedRecords) with structured outcome reporting (SplitResult exchange property), and resume/watermark tracking based on Camel’s existing ResumeStrategy SPI. The change spans core processor behavior, model/reifier wiring, YAML/XML IO/schema generation, documentation, and extensive new unit tests.
Changes:
- Add new Splitter DSL/model options for chunking, error thresholds, and resume/watermark configuration (Java/XML/YAML).
- Implement failure tracking + abort policies and persist a
SplitResultexchange property when thresholds are enabled. - Implement index-based and value-based watermark persistence/restore via
ResumeStrategy, plus docs and tests covering streaming/parallel/transacted paths.
Reviewed changes
Copilot reviewed 18 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| dsl/camel-yaml-dsl/camel-yaml-dsl/src/generated/resources/schema/camelYamlDsl.json | YAML DSL schema updates for new Split options. |
| dsl/camel-yaml-dsl/camel-yaml-dsl-deserializers/src/generated/java/org/apache/camel/dsl/yaml/deserializers/ModelDeserializers.java | YAML deserializer wiring for new SplitDefinition fields. |
| core/camel-yaml-io/src/generated/java/org/apache/camel/yaml/out/YamlModelWriter.java | YAML writer emits new SplitDefinition attributes. |
| core/camel-xml-io/src/generated/java/org/apache/camel/xml/out/ModelWriter.java | XML writer emits new SplitDefinition attributes. |
| core/camel-xml-io/src/generated/java/org/apache/camel/xml/in/ModelParser.java | XML parser reads new SplitDefinition attributes. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterWatermarkTest.java | Tests for index/value watermark behavior, including group + parallel scenarios. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterValidationTest.java | Validation tests for threshold ranges and resume/watermark config completeness. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterTransactedTest.java | Coverage for new features via the transacted Multicast code path. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterTestResumeStrategy.java | Test ResumeStrategy implementation used by watermark-related tests. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterStreamingTest.java | Coverage for streaming mode interactions with thresholds/grouping/watermark. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterSplitResultTest.java | Tests for SplitResult contents and presence/absence rules. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterParallelErrorThresholdTest.java | Parallel-mode behavior tests for abort thresholds. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterMaxFailedRecordsTest.java | Functional tests for max-failed abort logic and exclusivity constraints. |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterGroupTest.java | Functional tests for chunking/grouping (including with failures). |
| core/camel-core/src/test/java/org/apache/camel/processor/SplitterErrorThresholdTest.java | Functional tests for ratio-based threshold behavior and boundaries. |
| core/camel-core-reifier/src/main/java/org/apache/camel/reifier/SplitReifier.java | Model-to-processor wiring and validation for new Split options. |
| core/camel-core-processor/src/main/java/org/apache/camel/processor/Splitter.java | Core implementation for grouping, threshold aborts, SplitResult, and watermark tracking. |
| core/camel-core-processor/src/main/java/org/apache/camel/processor/MulticastProcessor.java | Extract shouldContinueOnFailure hook to allow Splitter-specific failure policy. |
| core/camel-core-model/src/main/java/org/apache/camel/model/SplitDefinition.java | New model fields + fluent DSL methods for group/threshold/watermark/resumeStrategy. |
| core/camel-core-model/src/generated/resources/META-INF/org/apache/camel/model/split.json | Regenerated model metadata for new SplitDefinition attributes and exchange properties. |
| core/camel-core-engine/src/main/docs/modules/eips/pages/split-eip.adoc | Documentation for chunking, error thresholds, SplitResult, and watermark usage/caveats. |
| core/camel-api/src/main/java/org/apache/camel/SplitResult.java | New public API type representing structured Splitter outcomes. |
| core/camel-api/src/main/java/org/apache/camel/ExchangePropertyKey.java | Adds typed exchange-property key support for the new SplitResult property. |
| core/camel-api/src/main/java/org/apache/camel/Exchange.java | Adds SPLIT_RESULT and SPLIT_WATERMARK exchange property constants. |
| core/camel-api/src/generated/java/org/apache/camel/ExchangeConstantProvider.java | Regenerated constant provider entries for new exchange property constants. |
| catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/schemas/camel-xml-io.xsd | Regenerated XML schema reflecting new SplitDefinition attributes. |
| catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/schemas/camel-spring.xsd | Regenerated Spring XML schema reflecting new SplitDefinition attributes. |
| catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/models/split.json | Regenerated catalog model metadata for new SplitDefinition options. |
Comments suppressed due to low confidence (1)
core/camel-api/src/main/java/org/apache/camel/ExchangePropertyKey.java:232
- After adding
SPLIT_WATERMARKtoExchangePropertyKey, it also needs to be handled inasExchangePropertyKey(String); otherwise typed lookups won’t resolve this new key.
case Exchange.SPLIT_SIZE:
return SPLIT_SIZE;
case Exchange.STEP_ID:
return STEP_ID;
case Exchange.STREAM_CACHE_UNIT_OF_WORK:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int group = parseInt(definition.getGroup(), 0); | ||
| if (group > 0) { | ||
| answer.setGroup(group); | ||
| } |
There was a problem hiding this comment.
Good catch — added validation to reject negative group values with IllegalArgumentException in SplitReifier.
Claude Code on behalf of Guillaume Nodet
| private void configureErrorThreshold(Splitter answer, boolean isStopOnException) { | ||
| String etStr = parseString(definition.getErrorThreshold()); | ||
| double errorThreshold = etStr != null ? Double.parseDouble(etStr) : 0; | ||
| int maxFailedRecords = parseInt(definition.getMaxFailedRecords(), 0); | ||
| boolean hasErrorThreshold = errorThreshold > 0 || maxFailedRecords > 0; | ||
| if (hasErrorThreshold && isStopOnException) { | ||
| throw new IllegalArgumentException( | ||
| "Cannot use both stopOnException and errorThreshold/maxFailedRecords on the Splitter EIP"); | ||
| } | ||
| if (errorThreshold != 0 && (errorThreshold < 0 || errorThreshold > 1.0)) { | ||
| throw new IllegalArgumentException( | ||
| "errorThreshold must be between 0.0 and 1.0, but was: " + errorThreshold); | ||
| } | ||
| if (maxFailedRecords < 0) { | ||
| throw new IllegalArgumentException( | ||
| "maxFailedRecords must not be negative, but was: " + maxFailedRecords); | ||
| } | ||
| if (errorThreshold > 0) { | ||
| answer.setErrorThreshold(errorThreshold); | ||
| } | ||
| if (maxFailedRecords > 0) { | ||
| answer.setMaxFailedRecords(maxFailedRecords); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is intentional. errorThreshold(0.0) means "tolerate zero failures" which is semantically identical to stopOnException(true). The > 0 check correctly treats 0.0 as "not configured" since users wanting that behavior should use stopOnException or maxFailedRecords(1) instead. The validation at line 103 allows 0.0 to pass without error (rather than throwing) because setting errorThreshold(0.0) is a no-op, not a misconfiguration worth rejecting.
Claude Code on behalf of Guillaume Nodet
| private boolean setupFailureTracking(Exchange exchange) { | ||
| boolean hasErrorThreshold = errorThreshold > 0 || maxFailedRecords > 0; | ||
| if (hasErrorThreshold) { | ||
| exchange.setProperty(SPLIT_FAILURE_TRACKER, new SplitFailureTracker()); | ||
| } |
There was a problem hiding this comment.
Same rationale as above — errorThreshold > 0 is intentional. errorThreshold(0.0) is a no-op (same as not setting it), since "tolerate zero failures" is what stopOnException is for. The > 0 check is consistent between SplitReifier (wiring) and Splitter (runtime).
Claude Code on behalf of Guillaume Nodet
| // don't update watermark if processing was aborted (allows retry) | ||
| if (exchange.getException() != null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Good catch — fixed. Moved the cleanup of internal properties (SPLIT_WATERMARK_OFFSET, SPLIT_WATERMARK_COUNT, SPLIT_WATERMARK_LATEST) into a finally block so they are always removed, even on abort. The watermark value itself is still not persisted on abort (intentional — allows retry from the same position).
Claude Code on behalf of Guillaume Nodet
| /** | ||
| * Sets a Simple expression to evaluate on the exchange after split completion to determine the new watermark value. | ||
| */ |
There was a problem hiding this comment.
Good catch — fixed. Updated the javadoc to: "Sets a Simple expression to evaluate on each completed sub-exchange to determine the new watermark value. When set, enables value-based watermarking instead of index-based."
This matches the actual implementation which evaluates the expression per-item via ProcessorExchangePair.done() and persists only the last value.
Claude Code on behalf of Guillaume Nodet
| SPLIT_COMPLETE(Exchange.SPLIT_COMPLETE), | ||
| SPLIT_INDEX(Exchange.SPLIT_INDEX), | ||
| SPLIT_RESULT(Exchange.SPLIT_RESULT), | ||
| SPLIT_SIZE(Exchange.SPLIT_SIZE), | ||
| STEP_ID(Exchange.STEP_ID), |
There was a problem hiding this comment.
Done — added SPLIT_WATERMARK(Exchange.SPLIT_WATERMARK) to the enum and the corresponding case in asExchangePropertyKey(). Same fix as requested by @davsclaus.
Claude Code on behalf of Guillaume Nodet
|
rebase on main |
47440bf to
630d2a1
Compare
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 555 tested, 26 compile-only — current: 552 all testedMaveniverse Scalpel detected 581 affected modules (current approach: 552).
|
ad43981 to
79bad2e
Compare
|
Done — rebased onto latest main (5 new commits since last push). Clean rebase, no conflicts. CI is running now. Claude Code on behalf of Guillaume Nodet |
… failure tracking - Add group(int) option to chunk split items into List batches using GroupIterator - Add errorThreshold(double) to abort when failure ratio exceeds threshold - Add maxFailedRecords(int) to abort after N failures - Add SplitResult exchange property with structured failure details - Extract shouldContinueOnFailure() in MulticastProcessor for subclass override - Mutually exclusive validation: stopOnException vs errorThreshold/maxFailedRecords Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Index-based watermarking: skip already-processed items on subsequent runs - Value-based watermarking: expose stored watermark as exchange property, evaluate Simple expression after completion to determine new value - Watermark is only updated on successful completion (not on abort) - Add SPLIT_WATERMARK exchange property constant Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Evaluate watermark expression per-item via ProcessorExchangePair.done() hook instead of requiring a custom aggregation strategy - Add WatermarkProcessorExchangePair wrapper for thread-safe per-item watermark tracking using AtomicReference - Add SplitterTransactedTest with 6 tests covering group, error threshold, split result, and watermark features using the MulticastTransactedTask code path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Regenerated: catalog split.json, Spring/XML-IO XSD schemas, XML/YAML ModelParser/ModelWriter, YAML DSL ModelDeserializers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix index-based watermark producing wrong offset when combined with group(): track raw item count independently of chunk index - Optimize double readCurrentWatermark() call by reusing exchange property - Refactor watermark from Map<String,String> to ResumeStrategy SPI - Add validation for errorThreshold range and watermarkKey/resumeStrategy completeness - Add documentation for all new features (group, errorThreshold, maxFailedRecords, SplitResult, watermark tracking) to split-eip.adoc - Document parallel processing limitations for errorThreshold - Document concurrent exchange limitations for watermark tracking - Add upgrade guide section for Split EIP enhancements - Add tests: group+watermark, parallel+errorThreshold, group+errorThreshold, streaming+parallel+maxFailedRecords, streaming mode, validation, and ResumeStrategy test helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ents - Clarify SplitResult.totalItems counts chunks (not individual items) when group() is used - Add LOG.debug in readFromStrategyCache() catch block for debuggability - Document that custom AggregationStrategy won't see individual item exceptions when error thresholds are configured (exceptions cleared after recording) - Update SplitResult code example to note chunk counting with group() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tegy rename Fix stale watermarkStore references in XSD schemas, ModelParser, and ModelWriter files that were generated before the field was renamed to resumeStrategy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r Splitter enhancements - Extract setupFailureTracking(), setupWatermarkTracking(), wrapCallback() from Splitter.process() - Extract configureErrorThreshold(), configureWatermark() from SplitReifier.createProcessor() - Add tests: SplitResult.toString(), null failures constructor, empty input, Failure record - Add tests: watermark expression returning null, all items failing, combined errorThreshold+watermark - Add tests: empty input with watermark, single item, value watermark no-update-on-abort - Add tests: error threshold boundary values, exact boundary, empty input Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… tags, regenerate - Remove Split EIP section from upgrade guide (davsclaus: new features don't belong in the upgrade guide, only migration info) - Add @SInCE 4.21 to SplitResult class and Failure record (oscerd) - Regenerate all downstream artifacts after rebase onto latest main Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix @SInCE 4.21 to @SInCE 4.22 on SplitResult and Failure record - Add @SInCE 4.22 to Exchange.SPLIT_RESULT and SPLIT_WATERMARK constants - Add SPLIT_WATERMARK to ExchangePropertyKey enum and asExchangePropertyKey() - Reject negative group values in SplitReifier with IllegalArgumentException - Fix watermark property leak on abort: clean up internal properties in finally block so they don't leak to downstream processors - Fix setWatermarkExpression javadoc to match actual per-item semantics - Regenerate camelYamlDsl.json with full descriptions from @metadata annotations (fixes CI "uncommitted changes" failure) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
79bad2e to
7778167
Compare


Summary
Enhances the Splitter EIP with four new features that were previously proposed as a standalone
camel-bulkcomponent (CAMEL-23240, PR #22159). After analysis, these features compose naturally with the existing Splitter and benefit from a first-class DSL integration rather than a separate component.New DSL options
group(int)— Chunks split items intoListbatches of N. Wraps the underlying iterator withGroupIterator. More discoverable than the existingcollate(n)Simple expression and works with any split expression.errorThreshold(double)— Aborts when the failure ratio exceeds the threshold (0.0–1.0). For example,errorThreshold(0.5)stops when more than 50% of items fail. Mutually exclusive withstopOnException. Note: withparallelProcessing, the ratio may vary between runs due to non-deterministic callback ordering; prefermaxFailedRecordsfor deterministic abort behavior in parallel mode.maxFailedRecords(int)— Aborts after N item failures. Mutually exclusive withstopOnException. Can be combined witherrorThreshold.SplitResultexchange property — When error thresholds are configured, a structuredSplitResultis set as an exchange property (CamelSplitResult) after split completion. ProvidestotalItems,failureCount,successCount, individualFailuredetails (index + exception), and anabortedflag.resumeStrategy(ResumeStrategy, key)— Enables resume-from-last-position for split operations using Camel's existingResumeStrategySPI:ResumeStrategy. Works correctly withgroup()— tracks raw item indices independently of chunk grouping.watermarkExpression(expr)): The expression (Simple language) is evaluated on each successfully processed sub-exchange as it completes (viaProcessorExchangePair.done()hook). The last evaluated value is stored as the new watermark. The previous watermark is exposed asCamelSplitWatermarkexchange property for upstream filtering.ResumeCacheon each exchange start, not just on init.Usage examples
Chunking — process items in batches of 100
Error threshold — tolerate up to 10% failures
Max failed records — stop after 5 failures
Index-based watermark — resume from last position
Value-based watermark — track last processed timestamp
Implementation approach
shouldContinueOnFailure()as a protected method inMulticastProcessor, replacing the inlinestopOnExceptioncheck. 100% backward-compatible: all existing EIPs (Multicast, RecipientList) behave identically.SplitteroverridesshouldContinueOnFailure()to track failures in a thread-safeSplitFailureTracker(usesAtomicInteger+CopyOnWriteArrayListfor parallel mode safety).ProcessorExchangePair.done()hook (thread-safe usingAtomicReference.accumulateAndGetwith index-based comparison for deterministic results in parallel mode).ResumeStrategySPI withOffsetKeys/Offsetsfor persistence, plus a localConcurrentHashMapcache for fast reads.GroupIteratorchunking via a counting decorator, ensuring correct watermark values whengroup()is used.SplitReifiervalidates: mutual exclusivity ofstopOnExceptionwith error thresholds,errorThresholdrange (0.0–1.0),maxFailedRecordsnon-negative, completeness ofresumeStrategy/watermarkKeyconfiguration.Splitter.process()andSplitReifier.createProcessor()by extracting helper methods.Files changed
SplitResult.javaFailurerecordExchange.javaSPLIT_RESULT,SPLIT_WATERMARKconstantsExchangePropertyKey.javaSPLIT_RESULT,SPLIT_WATERMARKenum entriesSplitDefinition.javagroup,errorThreshold,maxFailedRecords,resumeStrategy,watermarkKey,watermarkExpressionfields + fluent methods with javadoc on parallel limitationsMulticastProcessor.javashouldContinueOnFailure()methodSplitter.javaResumeStrategy, raw item counting for group+watermark correctness, extracted helper methods for reduced complexitySplitReifier.javaResumeStrategylookup, error threshold range, config completeness; extractedconfigureErrorThreshold()andconfigureWatermark()ModelDeserializers.java,camelYamlDsl.jsonsplit-eip.adoccamel-4x-upgrade-guide-4_19.adocTest plan
SplitterGroupTest— 5 tests: group=3 with 7 items, exact multiple, single item, parallel processing, group+maxFailedRecordsSplitterMaxFailedRecordsTest— 5 tests: stops after threshold, all succeed, single failure (below threshold), mutual exclusivity with stopOnException, parallel processingSplitterErrorThresholdTest— 10 tests: stops when ratio exceeded, below ratio continues, all succeed, mutual exclusivity with stopOnException, parallel processing, combined thresholds, empty input, exact boundary, just-below boundary, maxFailedRecords exact boundarySplitterSplitResultTest— 11 tests: result with failures, result when aborted, all success, streaming mode, parallel mode, no result without threshold, toString() format, toString() aborted, null failures constructor, empty input, Failure recordSplitterWatermarkTest— 15 tests: index-based first/second run, no update on abort, value-based per-item/with previous, parallel value/index, group+watermark first/second run, null expression result, all items fail, combined errorThreshold+watermark, empty input, single item, value no-update-on-abortSplitterTransactedTest— 6 tests: all new features via theMulticastTransactedTaskcode pathSplitterStreamingTest— 6 tests: streaming SplitResult total items, streaming abort, streaming grouping, streaming watermark first/second run, streaming+parallel+maxFailedRecordsSplitterParallelErrorThresholdTest— 3 tests: parallel+maxFailedRecords abort, parallel all succeed, parallel+errorThreshold high failure rateSplitterValidationTest— 6 tests: errorThreshold negative/above-1 rejected, maxFailedRecords negative rejected, watermarkKey without resumeStrategy rejected, resumeStrategy without key rejected, watermarkExpression without resumeStrategy rejectedSupersedes #22159 (camel-bulk component, closed).
JIRA: https://issues.apache.org/jira/browse/CAMEL-23264